[16.0][FIX] prevent a warning message #644
Conversation
|
Hi @guewen, |
|
This PR has the |
queue_job/controllers/main.py
Outdated
| @@ -160,7 +160,7 @@ def _get_failure_values(self, job, traceback_txt, orig_exception): | |||
| exception_name = orig_exception.__class__.__name__ | |||
| if hasattr(orig_exception, "__module__"): | |||
| exception_name = orig_exception.__module__ + "." + exception_name | |||
| exc_message = getattr(orig_exception, "name", str(orig_exception)) | |||
| exc_message = getattr(orig_exception, "args[0]", str(orig_exception)) | |||
There was a problem hiding this comment.
Are you sure the this instance has an attribute called "args[0]"? This seems broken to me 🤔
args will be always there and very likely you'll have a first arg.
I guess you want to have something like orig_exception.args[0] if orig_exception.args else str(orig_exception).
There was a problem hiding this comment.
Per 8.3 Handling Exceptions 1,
The except clause may specify a variable after the exception name. The variable is bound to the exception instance which typically has an args attribute that stores the arguments. For convenience, builtin exception types define str() to print all the arguments without explicitly accessing .args.
"Typically" suggests that args will be absent some of the time.
- At the risk of having a slightly uglier error message, why not just use
str(orig_exception)instead of trying to preserve formatting? - If the exception handling behavior is changing, it should be covered by unit tests.
Footnotes
There was a problem hiding this comment.
Indeed the syntax looks weird so your proposal would be better. However, @amh-mw made a point so I think it would be better to simply use str(orig_exception).
There was a problem hiding this comment.
Maybe I can give you a little background as I'm the author of this piece of code 😉
The aim was to clearly rip out of the msg the exception class. That's why we have exc_name and exc_message. Exceptions are already messy for end users and this is an attempt to have a better msg.
Not to mention that this proposal will cause duplication of info because the exc name will be repeated.
Hence, calling str was just a safe fallback to make sure you had a msg.
An alternative is to call str and drop surrounding chars like exc name, parentesis, etc but I find this more complex than the little check here.
benwillig
force-pushed
the
16.0-fix_exception_name_to_args0-bwi
branch
from
716304d
to
43d0e46
Compare
'name' attributes for odoo's exception has been deprecated and produces a warning message. This commit fixes the behavior
benwillig
force-pushed
the
16.0-fix_exception_name_to_args0-bwi
branch
from
43d0e46
to
8479b13
Compare
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
github-actions
bot
added
the
stale
It doesn't make sense and even more, it crashed. Fixes OCA#644
Use args[0] instead of name to get the exception message as the name attribute is deprecated